Skip to content

updating the code to work with python 3.12#18

Closed
gavvahar wants to merge 4 commits intocazier:mainfrom
gavvahar:master
Closed

updating the code to work with python 3.12#18
gavvahar wants to merge 4 commits intocazier:mainfrom
gavvahar:master

Conversation

@gavvahar
Copy link
Contributor

No description provided.

Copy link
Owner

@cazier cazier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking at this old project of mine @gavvahar! It's long been on my todo list to look into again myself.


# Pyre type checker
.pyre/
.lh
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's pretty harmless just in the .gitignore, but what is this for? I think this can be removed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole webapp doesn't have any external services, so I don't see a need for it to have a docker compose. I appreciate the Dockerfile though!

Flask==1.1.2
Flask-SocketIO==5.0.1
greenlet==1.0.0
greenlet
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency (and many others) is out of date, but I don't want to unpin them.

if __name__ == '__main__':
socketio.run(app, debug=True, host=u'0.0.0.0')
if __name__ == "__main__":
socketio.run(app, debug=False, host="0.0.0.0")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
socketio.run(app, debug=False, host="0.0.0.0")
socketio.run(app, debug=True, host="0.0.0.0")

This should all probably be reading in parameters from a config file or something, but let's keep the original default value.

Comment on lines +14 to +17
COPY requirements.txt ./
RUN python3 -m pip install --no-cache-dir -r requirements.txt

COPY . .
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use this last COPY you don't need both. It may be easier for development to save the cached layer for later, but it doesn't need to be that way on the repo.

@cazier
Copy link
Owner

cazier commented Jul 4, 2025

Thanks, again, for all your help. I've merged in your changes, with my comments in #23.

@cazier cazier closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants